Support HTTP/2 after proxy CONNECT#2250
Conversation
536b173 to
e14569c
Compare
hyperxpro
left a comment
There was a problem hiding this comment.
Not in this PR's diff, so leaving it here: this change makes HTTP/2 after CONNECT work, which makes the workaround in client/src/test/java/org/asynchttpclient/proxy/HttpsProxyTestcontainersIntegrationTest.java stale and its comment misleading. Could you remove .setHttp2Enabled(false) and the // HTTP/2 ALPN upgrade after proxy CONNECT tunnel is not yet supported comment in both testHttpProxyToHttpsTarget (lines 149-150) and testHttpsProxyToHttpsTarget (lines 172-173)? That lets those external tests exercise the new HTTP/2-through-proxy path instead of forcing HTTP/1.1, and the comment stops claiming something that is no longer true.
|
Done - removed |
|
Hi @hyperxpro, just a gentle reminder when you get a chance. I've addressed the review comments and would appreciate a re-review. Thanks! |
|
Thanks a lot! |
|
Hey @hyperxpro - thanks again for reviewing and merging this! Could you share how the release process usually works for this project? Will this change be included in the next release, and is there a rough timeline for it? |
Motivation
When an HTTPS request uses an HTTP proxy, AHC establishes a
CONNECTtunnel and then performs TLS with the target. If ALPN selectsh2, the tunnel path currently leaves the HTTP/1.1 codec in place. The target then receives HTTP/1.1 bytes on a connection that negotiated HTTP/2 and closes it.Modification
h2, upgrade the existing channel pipeline to HTTP/2 and register the physical tunnel connection with its existing pool partition key.CONNECTproxies with a TLS HTTP/2 target.Result
HTTPS requests through HTTP and HTTPS
CONNECTproxies now send HTTP/2 frames when the target negotiatesh2. The regression tests verify successful HTTP/2 responses and reuse of a single physical target connection.Testing
./mvnw -pl client -Dtest=BasicHttp2Test,HttpsProxyTest test— 60 tests passed.verify; the local run then stopped at artifact signing becausegpgis not installed../mvnw -pl client -DskipTests -Dgpg.skip=true verify— packaging and API compatibility checks passed.Fixes #2241